Skip to content

Fix SystemChat#33

Open
Pantera07 wants to merge 1 commit intoTerraByteDev:mainfrom
Pantera07:fix/system-chat
Open

Fix SystemChat#33
Pantera07 wants to merge 1 commit intoTerraByteDev:mainfrom
Pantera07:fix/system-chat

Conversation

@Pantera07
Copy link

@Pantera07 Pantera07 commented Jan 8, 2026 •
edited by coderabbitai bot
Loading

Fixes #30, Fixes #31

Summary by CodeRabbit

  • Bug Fixes
    • Refined system chat message packet handling behavior to improve message processing reliability and accuracy.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 8, 2026 •
edited
Loading

📝 Walkthrough

Walkthrough

The PR modifies the packet handler for outgoing chat messages, removing sender resolution for system chat messages and adjusting the PacketAccessorResult classification flag. This fixes incorrect message attribution where viewers' data was displayed instead of the message sender's information.

Changes

Cohort / File(s) Summary
Chat Packet Handler Fix
src/main/java/net/skullian/platform/PacketEventsOutMessagePacketHelper.java		 Removed sender resolution lambda for SYSTEM_CHAT_MESSAGE registration; changed PacketAccessorResult flag from true to false for system message classification

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A message was lost, showing the wrong face,
The viewer's own data in chat's sacred space,
But now with a flag flip, the sender shines true,
No more mixed-up inventory—hooray, we're through! 🎉

🚥 Pre-merge checks | ✅ 3 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Fix SystemChat' is vague and non-descriptive. It uses a generic placeholder term without clearly conveying what specific aspect of SystemChat is being fixed or what the main change entails. Provide a more specific title that describes the core fix, such as 'Fix SystemChat sender data resolution in packet handler' or 'Resolve SystemChat packet accessor flag issue'.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed The changes address the core issues by removing sender resolution lambda and changing the PacketAccessorResult flag, directly targeting the sender data rendering problem in issues #30 and #31.
Out of Scope Changes check ✅ Passed All changes are scoped to the SYSTEM_CHAT_MESSAGE packet handler and directly address the linked issues; no extraneous modifications are present.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dade411 and 75e9ad0.

📒 Files selected for processing (1)
  • src/main/java/net/skullian/platform/PacketEventsOutMessagePacketHelper.java
🔇 Additional comments (1)
src/main/java/net/skullian/platform/PacketEventsOutMessagePacketHelper.java (1)

193-217: Remove sender determination from SYSTEM_CHAT_MESSAGE handler.

The change to use the simpler PacketEventsHandler constructor (5 parameters, without DeterminedSenderFinder) is appropriate for system chat messages, which are server-generated and lack a specific player sender. This prevents incorrect sender attribution by using false for the fourth parameter in PacketAccessorResult, consistent with other non-player-specific message handlers like SET_TITLE_TEXT and ACTION_BAR.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

The click timeout inventory doesn't work [inv] [item] ... shows viewer's data instead of sender's one

1 participant

@Pantera07